[lldb-dap] Updating protocol memory references to lldb::addr_t.
#148037
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This updates all the existing memory reference fields to use `lldb::addr_t` directly. A few places were using `std::string` and decoding the value in the request handler and other places had unique ways of parsing addresses. This unifies all of these references with the `DecodeMemoryReference` helper in JSONUtils.h. Additionally, for the types I updated, I tried to simplify the POD types some and moved default values out of RequestHandlers and into the protocol POD types.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThis updates all the existing memory reference fields to use A few places were using This unifies all of these references with the Additionally, for the types I updated, I tried to simplify the POD types some and moved default values out of RequestHandlers and into the protocol POD types. Patch is 22.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148037.diff 11 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 68f58bf1349a7..964d685e61161 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -831,13 +831,17 @@ def request_readMemory(self, memoryReference, offset, count):
}
return self.send_recv(command_dict)
- def request_writeMemory(self, memoryReference, data, offset=0, allowPartial=True):
+ def request_writeMemory(self, memoryReference, data, offset=0, allowPartial=False):
args_dict = {
"memoryReference": memoryReference,
- "offset": offset,
- "allowPartial": allowPartial,
"data": data,
}
+
+ if offset:
+ args_dict["offset"] = offset
+ if allowPartial:
+ args_dict["allowPartial"] = allowPartial
+
command_dict = {
"command": "writeMemory",
"type": "request",
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 00b01d4bdb1c5..d823126e3e2fd 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -508,7 +508,7 @@ def getBuiltinDebugServerTool(self):
self.assertIsNotNone(server_tool, "debugserver not found.")
return server_tool
- def writeMemory(self, memoryReference, data=None, offset=None, allowPartial=None):
+ def writeMemory(self, memoryReference, data=None, offset=0, allowPartial=False):
# This function accepts data in decimal and hexadecimal format,
# converts it to a Base64 string, and send it to the DAP,
# which expects Base64 encoded data.
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 69d4eab0b6fcd..a3a4bdaaf40a6 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -141,8 +141,7 @@ def do_test_scopes_variables_setVariable_evaluate(
self, enableAutoVariableSummaries: bool
):
"""
- Tests the "scopes", "variables", "setVariable", and "evaluate"
- packets.
+ Tests the "scopes", "variables", "setVariable", and "evaluate" packets.
"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(
@@ -304,7 +303,6 @@ def do_test_scopes_variables_setVariable_evaluate(
verify_response = {
"type": "int",
"value": str(variable_value),
- "variablesReference": 0,
}
for key, value in verify_response.items():
self.assertEqual(value, response["body"][key])
@@ -723,7 +721,7 @@ def test_indexedVariables_with_raw_child_for_synthetics(self):
self.do_test_indexedVariables(enableSyntheticChildDebugging=True)
@skipIfWindows
- @skipIfAsan # FIXME this fails with a non-asan issue on green dragon.
+ @skipIfAsan # FIXME this fails with a non-asan issue on green dragon.
def test_registers(self):
"""
Test that registers whose byte size is the size of a pointer on
diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index f66c87fa9893d..9542f091b055e 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -182,14 +182,7 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
/// `supportsDisassembleRequest` is true.
llvm::Expected<DisassembleResponseBody>
DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
- std::optional<lldb::addr_t> addr_opt =
- DecodeMemoryReference(args.memoryReference);
- if (!addr_opt.has_value())
- return llvm::make_error<DAPError>("Malformed memory reference: " +
- args.memoryReference);
-
- lldb::addr_t addr_ptr = *addr_opt;
- addr_ptr += args.offset.value_or(0);
+ const lldb::addr_t addr_ptr = args.memoryReference + args.offset;
lldb::SBAddress addr(addr_ptr, dap.target);
if (!addr.IsValid())
return llvm::make_error<DAPError>(
@@ -197,7 +190,7 @@ DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
// Offset (in instructions) to be applied after the byte offset (if any)
// before disassembling. Can be negative.
- int64_t instruction_offset = args.instructionOffset.value_or(0);
+ const int64_t instruction_offset = args.instructionOffset;
// Calculate a sufficient address to start disassembling from.
lldb::SBAddress disassemble_start_addr =
@@ -212,8 +205,8 @@ DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
return llvm::make_error<DAPError>(
"Unexpected error while disassembling instructions.");
- // Conver the found instructions to the DAP format.
- const bool resolve_symbols = args.resolveSymbols.value_or(false);
+ // Convert the found instructions to the DAP format.
+ const bool resolve_symbols = args.resolveSymbols;
std::vector<DisassembledInstruction> instructions;
size_t original_address_index = args.instructionCount;
for (size_t i = 0; i < insts.GetSize(); ++i) {
diff --git a/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp
index 7065b6a24b554..374dc4516aa2d 100644
--- a/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp
@@ -37,7 +37,7 @@ ReadMemoryRequestHandler::Run(const protocol::ReadMemoryArguments &args) const {
const size_t memory_count = dap.target.GetProcess().ReadMemory(
raw_address, buffer.data(), buffer.size(), error);
- response.address = "0x" + llvm::utohexstr(raw_address);
+ response.address = raw_address;
// reading memory may fail for multiple reasons. memory not readable,
// reading out of memory range and gaps in memory. return from
diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
index 5b8ff563968f0..d07c0d6c9afa8 100644
--- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
@@ -68,14 +68,11 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
body.indexedVariables = variable.GetNumChildren();
else
body.namedVariables = variable.GetNumChildren();
-
- } else {
- body.variablesReference = 0;
}
if (const lldb::addr_t addr = variable.GetLoadAddress();
addr != LLDB_INVALID_ADDRESS)
- body.memoryReference = EncodeMemoryReference(addr);
+ body.memoryReference = addr;
if (ValuePointsToCode(variable))
body.valueLocationReference = new_var_ref;
diff --git a/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
index 7bd1c9f91ff47..313f59dceab24 100644
--- a/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
@@ -22,8 +22,7 @@ namespace lldb_dap {
llvm::Expected<protocol::WriteMemoryResponseBody>
WriteMemoryRequestHandler::Run(
const protocol::WriteMemoryArguments &args) const {
- const lldb::addr_t address = args.memoryReference + args.offset.value_or(0);
- ;
+ const lldb::addr_t address = args.memoryReference + args.offset;
lldb::SBProcess process = dap.target.GetProcess();
if (!lldb::SBDebugger::StateIsStoppedState(process.GetState()))
@@ -54,7 +53,7 @@ WriteMemoryRequestHandler::Run(
// If 'allowPartial' is false or missing, a debug adapter should attempt to
// verify the region is writable before writing, and fail the response if it
// is not.
- if (!args.allowPartial.value_or(false)) {
+ if (!args.allowPartial) {
// Start checking from the initial write address.
lldb::addr_t start_address = address;
// Compute the end of the write range.
@@ -74,7 +73,7 @@ WriteMemoryRequestHandler::Run(
"Memory 0x" + llvm::utohexstr(args.memoryReference) +
" region is not writable");
}
- // If the current region covers the full requested range, stop futher
+ // If the current region covers the full requested range, stop further
// iterations.
if (end_address <= region_info.GetRegionEnd()) {
break;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index e840677cdebd5..cc29c1c8c9620 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -8,6 +8,7 @@
#include "Protocol/ProtocolRequests.h"
#include "JSONUtils.h"
+#include "lldb/lldb-defines.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
@@ -311,26 +312,24 @@ bool fromJSON(const json::Value &Params, SetVariableArguments &SVA,
json::Value toJSON(const SetVariableResponseBody &SVR) {
json::Object Body{{"value", SVR.value}};
- if (SVR.type.has_value())
- Body.insert({"type", SVR.type});
- if (SVR.variablesReference.has_value())
+ if (!SVR.type.empty())
+ Body.insert({"type", SVR.type});
+ if (SVR.variablesReference)
Body.insert({"variablesReference", SVR.variablesReference});
-
- if (SVR.namedVariables.has_value())
+ if (SVR.namedVariables)
Body.insert({"namedVariables", SVR.namedVariables});
-
- if (SVR.indexedVariables.has_value())
+ if (SVR.indexedVariables)
Body.insert({"indexedVariables", SVR.indexedVariables});
-
- if (SVR.memoryReference.has_value())
- Body.insert({"memoryReference", SVR.memoryReference});
-
- if (SVR.valueLocationReference.has_value())
+ if (SVR.memoryReference != LLDB_INVALID_ADDRESS)
+ Body.insert(
+ {"memoryReference", EncodeMemoryReference(SVR.memoryReference)});
+ if (SVR.valueLocationReference)
Body.insert({"valueLocationReference", SVR.valueLocationReference});
return json::Value(std::move(Body));
}
+
bool fromJSON(const json::Value &Params, ScopesArguments &SCA, json::Path P) {
json::ObjectMapper O(Params, P);
return O && O.map("frameId", SCA.frameId);
@@ -471,7 +470,9 @@ json::Value toJSON(const ThreadsResponseBody &TR) {
bool fromJSON(const llvm::json::Value &Params, DisassembleArguments &DA,
llvm::json::Path P) {
json::ObjectMapper O(Params, P);
- return O && O.map("memoryReference", DA.memoryReference) &&
+ return O &&
+ DecodeMemoryReference(Params, "memoryReference", DA.memoryReference, P,
+ /*required=*/true) &&
O.mapOptional("offset", DA.offset) &&
O.mapOptional("instructionOffset", DA.instructionOffset) &&
O.map("instructionCount", DA.instructionCount) &&
@@ -485,29 +486,14 @@ json::Value toJSON(const DisassembleResponseBody &DRB) {
bool fromJSON(const json::Value &Params, ReadMemoryArguments &RMA,
json::Path P) {
json::ObjectMapper O(Params, P);
-
- const json::Object *rma_obj = Params.getAsObject();
- constexpr llvm::StringRef ref_key = "memoryReference";
- const std::optional<llvm::StringRef> memory_ref = rma_obj->getString(ref_key);
- if (!memory_ref) {
- P.field(ref_key).report("missing value");
- return false;
- }
-
- const std::optional<lldb::addr_t> addr_opt =
- DecodeMemoryReference(*memory_ref);
- if (!addr_opt) {
- P.field(ref_key).report("Malformed memory reference");
- return false;
- }
-
- RMA.memoryReference = *addr_opt;
-
- return O && O.map("count", RMA.count) && O.mapOptional("offset", RMA.offset);
+ return O &&
+ DecodeMemoryReference(Params, "memoryReference", RMA.memoryReference,
+ P, /*required=*/true) &&
+ O.map("count", RMA.count) && O.mapOptional("offset", RMA.offset);
}
json::Value toJSON(const ReadMemoryResponseBody &RMR) {
- json::Object result{{"address", RMR.address}};
+ json::Object result{{"address", EncodeMemoryReference(RMR.address)}};
if (RMR.unreadableBytes != 0)
result.insert({"unreadableBytes", RMR.unreadableBytes});
@@ -570,24 +556,10 @@ bool fromJSON(const json::Value &Params, WriteMemoryArguments &WMA,
json::Path P) {
json::ObjectMapper O(Params, P);
- const json::Object *wma_obj = Params.getAsObject();
- constexpr llvm::StringRef ref_key = "memoryReference";
- const std::optional<llvm::StringRef> memory_ref = wma_obj->getString(ref_key);
- if (!memory_ref) {
- P.field(ref_key).report("missing value");
- return false;
- }
-
- const std::optional<lldb::addr_t> addr_opt =
- DecodeMemoryReference(*memory_ref);
- if (!addr_opt) {
- P.field(ref_key).report("Malformed memory reference");
- return false;
- }
-
- WMA.memoryReference = *addr_opt;
-
- return O && O.mapOptional("allowPartial", WMA.allowPartial) &&
+ return O &&
+ DecodeMemoryReference(Params, "memoryReference", WMA.memoryReference,
+ P, /*required=*/true) &&
+ O.mapOptional("allowPartial", WMA.allowPartial) &&
O.mapOptional("offset", WMA.offset) && O.map("data", WMA.data);
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 63f56fe36f148..0cfd9606b7969 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -398,13 +398,12 @@ bool fromJSON(const llvm::json::Value &, SetVariableArguments &,
/// Response to `setVariable` request.
struct SetVariableResponseBody {
-
/// The new value of the variable.
std::string value;
/// The type of the new value. Typically shown in the UI when hovering over
/// the value.
- std::optional<std::string> type;
+ std::string type;
/// If `variablesReference` is > 0, the new value is structured and its
/// children can be retrieved by passing `variablesReference` to the
@@ -414,26 +413,26 @@ struct SetVariableResponseBody {
/// If this property is included in the response, any `variablesReference`
/// previously associated with the updated variable, and those of its
/// children, are no longer valid.
- std::optional<uint64_t> variablesReference;
+ uint64_t variablesReference = 0;
/// The number of named child variables.
/// The client can use this information to present the variables in a paged
/// UI and fetch them in chunks.
/// The value should be less than or equal to 2147483647 (2^31-1).
- std::optional<uint32_t> namedVariables;
+ uint32_t namedVariables = 0;
/// The number of indexed child variables.
/// The client can use this information to present the variables in a paged
/// UI and fetch them in chunks.
/// The value should be less than or equal to 2147483647 (2^31-1).
- std::optional<uint32_t> indexedVariables;
+ uint32_t indexedVariables = 0;
/// A memory reference to a location appropriate for this result.
/// For pointer type eval results, this is generally a reference to the
/// memory address contained in the pointer.
/// This attribute may be returned by a debug adapter if corresponding
/// capability `supportsMemoryReferences` is true.
- std::optional<std::string> memoryReference;
+ lldb::addr_t memoryReference = LLDB_INVALID_ADDRESS;
/// A reference that allows the client to request the location where the new
/// value is declared. For example, if the new value is function pointer, the
@@ -442,7 +441,7 @@ struct SetVariableResponseBody {
///
/// This reference shares the same lifetime as the `variablesReference`. See
/// 'Lifetime of Object References' in the Overview section for details.
- std::optional<uint64_t> valueLocationReference;
+ uint64_t valueLocationReference = 0;
};
llvm::json::Value toJSON(const SetVariableResponseBody &);
@@ -805,26 +804,26 @@ llvm::json::Value toJSON(const SetExceptionBreakpointsResponseBody &);
struct DisassembleArguments {
/// Memory reference to the base location containing the instructions to
/// disassemble.
- std::string memoryReference;
+ lldb::addr_t memoryReference = LLDB_INVALID_ADDRESS;
/// Offset (in bytes) to be applied to the reference location before
/// disassembling. Can be negative.
- std::optional<int64_t> offset;
+ int64_t offset = 0;
/// Offset (in instructions) to be applied after the byte offset (if any)
/// before disassembling. Can be negative.
- std::optional<int64_t> instructionOffset;
+ int64_t instructionOffset = 0;
/// Number of instructions to disassemble starting at the specified location
/// and offset.
/// An adapter must return exactly this number of instructions - any
/// unavailable instructions should be replaced with an implementation-defined
/// 'invalid instruction' value.
- uint32_t instructionCount;
+ uint32_t instructionCount = 0;
/// If true, the adapter should attempt to resolve memory addresses and other
/// values to symbolic names.
- std::optional<bool> resolveSymbols;
+ bool resolveSymbols = false;
};
bool fromJSON(const llvm::json::Value &, DisassembleArguments &,
llvm::json::Path);
@@ -842,14 +841,14 @@ llvm::json::Value toJSON(const DisassembleResponseBody &);
/// Arguments for `readMemory` request.
struct ReadMemoryArguments {
/// Memory reference to the base location from which data should be read.
- lldb::addr_t memoryReference;
+ lldb::addr_t memoryReference = LLDB_INVALID_ADDRESS;
/// Offset (in bytes) to be applied to the reference location before reading
/// data. Can be negative.
int64_t offset = 0;
/// Number of bytes to read at the specified location and offset.
- uint64_t count;
+ uint64_t count = 0;
};
bool fromJSON(const llvm::json::Value &, ReadMemoryArguments &,
llvm::json::Path);
@@ -859,7 +858,7 @@ struct ReadMemoryResponseBody {
/// The address of the first byte of data returned.
/// Treated as a hex value if prefixed with `0x`, or as a decimal value
/// otherwise.
- std::string address;
+ lldb::addr_t address = LLDB_INVALID_ADDRESS;
/// The number of unreadable bytes encountered after the last successfully
/// read byte.
@@ -947,11 +946,11 @@ llvm::json::Value toJSON(const VariablesResponseBody &);
/// Arguments for `writeMemory` request.
struct WriteMemoryArguments {
/// Memory reference to the base location to which data should be written.
- lldb::addr_t memoryReference;
+ lldb::addr_t memoryReference = LLDB_INVALID_ADDRESS;
/// Offset (in bytes) to be applied to the reference location before writing
/// data. Can be negative.
- std::optional<int64_t> offset;
+ int64_t offset = 0;
/// Property to control partial writes. If true, the debug adapter should
/// attempt to write memory even if the entire memory region is not writable.
@@ -960,7 +959,7 @@ struct WriteMemoryArguments {
/// the response via the `offset` and `bytesWritten` properties.
/// If false or missing, a debug adapter should attempt to verify the region
/// is writable before writing, and fail the response if it is not.
- std::optional<bool> allowPartial;
+ bool allowPartial = false;
/// Bytes to write, encoded using base64.
std::string data;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index 785830c693104..fe8bb5252cc23 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -875,19 +875,10 @@ llvm::json::Value toJSON(const DisassembledInstruction::PresentationHint &PH) {
bool fromJSON(const llvm::json::Value &Params, DisassembledInstruction &DI,
llvm::json::Path P) {
llvm::json::ObjectMapper O(Params, P);
- std::string raw_address;
- if (!O || !O.map("address", raw_address))
- return false;
-
- std::optional<lldb::addr_t> address = DecodeMemoryReference(raw_address);
- if (!address) {
- P.field("address").rep...
[truncated]
|
da-viper
approved these changes
Jul 10, 2025
JDevlieghere
approved these changes
Jul 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates all the existing memory reference fields to use
lldb::addr_tdirectly.A few places were using
std::stringand decoding the value in the request handler and other places had unique ways of parsing addresses.This unifies all of these references with the
DecodeMemoryReferencehelper in JSONUtils.h.Additionally, for the types I updated, I tried to simplify the POD types some and moved default values out of RequestHandlers and into the protocol POD types.